-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move cugraph projects around for 24.12 #417
Conversation
features/src/rapids-build-utils/opt/rapids-build-utils/manifest.yaml
Outdated
Show resolved
Hide resolved
@@ -243,6 +222,35 @@ repos: | |||
sub_dir: python/cugraph-service/server | |||
args: {install: *rapids_build_backend_args} | |||
|
|||
- name: cugraph-gnn | |||
path: cugraph-gnn | |||
git: {host: github, upstream: jameslamb, repo: cugraph-gnn, tag: devcontainers} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this to test changes in rapidsai/cugraph-gnn#68
It should be pointed at the real upstream repo after that PR is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks to me like the changes in rapidsai/cugraph-gnn#68 did resolve the conda solve issues I was seeing before.
Now CI is failing here with what looks like a cuVS issues that's being fixed upstream: #418 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this is failing when compiling RAFT:
/home/coder/raft/cpp/bench/ann/src/faiss/faiss_gpu_wrapper.h(229): error: class "faiss::gpu::StandardGpuResources" has no member "getRaftHandle"
/home/coder/raft/cpp/bench/ann/src/faiss/faiss_gpu_wrapper.h(231): error: class "faiss::gpu::StandardGpuResources" has no member "getRaftHandle"
/home/coder/raft/cpp/bench/ann/src/faiss/faiss_gpu_wrapper.h(246): error: class "faiss::gpu::StandardGpuResources" has no member "getRaftHandle"
/home/coder/raft/cpp/bench/ann/src/faiss/faiss_gpu_wrapper.h(328): error: class "faiss::gpu::GpuIndexIVFFlatConfig" has no member "use_raft"
/home/coder/raft/cpp/bench/ann/src/faiss/faiss_gpu_wrapper.h(372): error: class "faiss::gpu::GpuIndexIVFPQConfig" has no member "use_raft"
5 errors detected in the compilation of "/home/coder/raft/cpp/bench/ann/src/faiss/faiss_gpu_benchmark.cu".
Maybe a result of facebookresearch/faiss#3549 being merged 2 hours ago?
@tarang-jain can you take a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR to hopefully fix the RAFT issue: rapidsai/raft#2496
Thanks for the reviews! Sorry, realize I'd left behind And while we're here and this is building, I also removed |
I believe this is needed to remove `nx-cugraph` from the `cugraph` repo being done in rapidsai/cugraph#4756 This is a subset of the changes @jameslamb is making in #417. Does it make sense to do this change for `nx-cugraph` and `cugraph-gnn` independently (how close is `cugraph-gnn` to being removed?)? Should we also add `nx-cugraph` as done in that PR? Anything else? @nv-rliu @jameslamb @rlratzel
🎉 this worked (build link)! Before merging, we need to merge rapidsai/cugraph-gnn#68 and then point this PR at https://github.com/rapidsai/cugraph-gnn (it's currently pointing at my fork for testing). |
Fixes some small `dependencies.yaml` issues to get devcontainers builds of these libraries working. Namely: * wholegraph needs NVML in its build environment * `pytorch-cuda` should be omitted when building on a CUDA minor version that it doesn't explicitly provide packages for ## Notes for Reviewers ### How I tested this Pointed rapidsai/devcontainers#417 at this branch and saw it pass. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #68
Adds devcontainers and devcontainer CI jobs. ## Notes for Reviewers I created this by copying from the `cugraph` repo and then just changing all the references to `nx-cugraph` (and removing a few unnecessary things, like details about cloning cugraph-ops). This will be blocked until rapidsai/devcontainers#417 or rapidsai/devcontainers#418 is merged. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Erik Welch (https://github.com/eriknw) - Paul Taylor (https://github.com/trxcllnt) - Ralph Liu (https://github.com/nv-rliu) - Bradley Dice (https://github.com/bdice) URL: #25
Follow-up to these PRs: * rapidsai/devcontainers#417 * #68 Proposes adding devcontainers and a devcontainers CI job to the repo. ## Notes for Reviewers ### Benefits of these changes * faster and easier local development * reduced risk of changes here breaking the RAPIDS unified devcontainers maintained in https://github.com/rapidsai/devcontainers Similar to rapidsai/nx-cugraph#25 ### How I made these changes Copied the `.devcontainer/` directory from https://github.com/rapidsai/cugraph, then just changed `cugraph` references to `cugraph-gnn`. ### How I tested this Tested the `update-version.sh` changes like this: ```shell ./ci/release/update-version.sh '25.04.00' git grep -E '25\.[0-9]+' ``` Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #79
Development of some
cugraph
projects is moving in 24.12.This updates the
rapids-build-utils
manifest to reflect those changes.Notes for Reviewers
The
nx-cugraph
changes ended up getting split off into their own PR: #418